-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FE] 행사 생성 페이지에 퍼널 방식 적용 #591
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
웨디 고생했어요~ 퍼널 구조로 바꿔준 덕분에 행사를 생성할 때 필드가 더 추가되어도 유연하게 추가할 수 있을 것 같아요!!
그리고 웨디가 cypress에서 url이 event/create/? 질문 떠오르는 것이 있긴한데 아마 form 태그의 submit 기본 동작때문에 그런 것이 아닐까 생각되는데 확인해보세요!
const navigate = useNavigate(); | ||
|
||
return ( | ||
<TextButton onClick={() => navigate(-1)} textSize="bodyBold" textColor="gray"> | ||
<TextButton onClick={() => (onClick ? onClick() : navigate(-1))} textSize="bodyBold" textColor="gray"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
back onClick의 기본값을 navigate(-1)기능을 넣어준 것은 컴포넌트의 이름이 back이기 때문에 ㄷ로가기 기능을 default로 넣어준걸까요?
// 행사 생성 페이지에서 여러 스텝에 걸쳐 사용되는 상태를 선언해 내려주는 용도의 훅입니다. | ||
const useCreateEventData = () => { | ||
const eventNameProps = useSetEventNameStep(); | ||
const [eventToken, setEventToken] = useState(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이벤트 토큰은 행사가 생성된 후 백엔드로부터 값을 받아오는 값이니깐 마지막 퍼널에서만 필요할 것 같아요.
즉 여러 상태에 걸쳐 사용되는 데이터는 아닌 것 같아서 다른 곳으로 이동해도 되지 않을까하는 생각입니다.
type UseFunnel = { | ||
defaultStep: string; | ||
stepList: string[]; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우리가 정의한 step과 여기의 step이 다른 의미라서 조금 헷갈릴 수 있을 것 같아요.
}; | ||
|
||
type FunnelProps = { | ||
children: React.ReactElement<StepProps>[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funnel안에 Step밖에 못 오게 한 것 넘 좋아잉
if (curStepIndex === stepList.length - 1) return; | ||
|
||
setStep(stepList[curStepIndex + 1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (curStepIndex === stepList.length - 1) return; | |
setStep(stepList[curStepIndex + 1]); | |
setStep(stepList[Math.min(stepList.length - 1, curStepIndex + 1)])); | |
if (curStepIndex === 0) return; | ||
|
||
setStep(stepList[curStepIndex - 1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (curStepIndex === 0) return; | |
setStep(stepList[curStepIndex - 1]); | |
setStep(stepList[Math.max(0, curStepIndex - 1)])); |
const targetStep = children.find(curStep => curStep.props.name === step); | ||
|
||
if (!targetStep) | ||
throw new Error(`현재 ${step} 단계에 보여줄 컴포넌트가 존재하지 않습니다. Step 컴포넌트를 호출해 사용해주세요.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
역시 웨디 에러까지 꼼꼼히 👍👍
if (validation.isValid) { | ||
setPassword(newValue); | ||
setErrorMessage(''); | ||
} else { | ||
event.target.value = password; | ||
setErrorMessage(validation.errorMessage ?? ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ErrorMessage type이 string | null 이었던 걸로 기억합니다. 그래서 빈 문자열 대신에 null을 넣는 것이 더 좋을 것 같아요! 하지만 중요한 것은 아니라 패쑤해도 괜찮아유
const handleBack = () => { | ||
if (step === STEP_SEQUENCE[0]) { | ||
navigate('/'); | ||
} else { | ||
moveToPrevStep(); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뒤로가기 눌렀을 때 바로 다 꺼지지 않고 전 스텝으로 돌리는 디테일 너무좋아요~
|
||
return ( | ||
<MainLayout backgroundColor="white"> | ||
<TopNav>{step !== STEP_SEQUENCE[STEP_SEQUENCE.length - 1] && <Back onClick={handleBack} />}</TopNav> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleBack에서 이미 뒤로가기 조건처리를 해주고 있는데 여기서 한 번 더 해준 이유가 있을까요?
issue
구현 이유
AddBillFunnel 이라는 퍼널 구현체(?)가 도입되었어요. 그래서 비슷하게 퍼널이 적용될 수 있는 곳인 행사 생성 기능도 퍼널을 적용했습니다.
구현 사항
처음에는 퍼널을 아래처럼 구현했어요.
하지만 하나의 스텝에서 다음 스텝으로 넘겨지는 함수(이후부터는
스텝넘기기 함수
라고 부르겠습니다.)가 이 컴포넌트 안에 작성되는게 보기 좋지 않아서useFunnel
을 구현하게 되었습니다.그런데 토스의 퍼널 영상을 보니까 훅 안에서 Funnel, Step이라는 컴포넌트도 return하고 있었습니다.
Funnel 컴포넌트는
현재 스텝에 해당하는 Step 컴포넌트를 찾아 렌더링
하는 책임이 있습니다. Step 컴포넌트는스텝의 이름과 children에 이 스텝에서 렌더링할 컴포넌트를 넘겨받기
라는 책임이 있었어요.그래서 스텝 넘기기 함수와 Funnel, Step을 담아 useFunnel을 구성하게 되었습니다.
그리고 최종적으로 아래와 같은 형태로 행사 생성 퍼널을 구현했어요.
하나의 스텝에서만 사용되는 것이 아닌 여러 스텝에 걸쳐 사용되는 상태는 하나의 훅(
useCreateEventData
)으로 묶고 행사 생성 퍼널 컴포넌트에서 호출해 하위 스텝으로 내려보내는 방식을 선택했습니다.이 퍼널에서만 사용되는 전역 상태를 만들어서 사용하는 방법도 있었는데요. 만약 스텝에 하위 컴포넌트가 많고, 거기까지 프롭이 내려가야 했다면 전역 상태를 사용했을 것 같은데 그렇게 깊진 않아서 그냥 드릴링으로 넘겨주게 되었어요.
논의하고 싶은 부분(선택)
행사 생성 페이지에서 여러 스텝에 걸쳐 사용되는 상태를 선언해 내려주는 용도의 훅인
useCreateEventData
의 이름이 직관적이지 않은 것 같은데 이름 추천 받습니다..테스트를 최대한 유지하고 싶어서 일단 바뀐 path를 사이프레스에 적용했는데요. cypress에서 행사 이름을 입력하고 "다음" 버튼을 누르면 퍼널이라서 비밀 번호 입력 페이지로 넘어가도 url이 바뀌면 안됩니다. 그런데
/create/event/?
경로로 바뀌네요.왜 이러는지 모르겠어요. 혹시 아시는 분 있나요?